Skip to content

fix(core): reset style fingerprint at each row boundary in diff renderer#1913

Merged
Karanjot786 merged 1 commit into
Karanjot786:mainfrom
ionfwsrijan:fix/style-fingerprint-row-boundary-1910
Jul 3, 2026
Merged

fix(core): reset style fingerprint at each row boundary in diff renderer#1913
Karanjot786 merged 1 commit into
Karanjot786:mainfrom
ionfwsrijan:fix/style-fingerprint-row-boundary-1910

Conversation

@ionfwsrijan

@ionfwsrijan ionfwsrijan commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Description

The differential renderer's style optimization (_lastStyleFingerprint) was only reset once per frame flush, not once per row. When _renderDiffLine renders spans on multiple rows, the style state bled across row boundaries because moveTo positions the cursor at a new row where the terminal has no memory of the previous cell's style. This caused sporadic missing ANSI style sequences, leading to visible color/bold/italic corruption on all rows after the first one.

Fix

Moved this._lastStyleFingerprint = null; inside the for (let r = 0; r < rows; r++) loop in _flush() (Renderer.ts:139-143) so the fingerprint is reset before each row is processed. This ensures the first cell on every row always emits the full ANSI reset+style sequence.

Test

Added 'resets style fingerprint at each row boundary in diff renderer' test that creates two rows with identical boundary styles and asserts the ANSI output contains at least two reset sequences (one per row).

Closes #1910

Summary by CodeRabbit

  • Bug Fixes
    • Fixed terminal diff rendering so style resets are correctly emitted at row boundaries.
    • Improved consistency when consecutive rows reuse the same styling, reducing missing or suppressed ANSI reset sequences.

@ionfwsrijan ionfwsrijan requested a review from Karanjot786 as a code owner June 30, 2026 16:41
@github-actions github-actions Bot added type:bug +10 pts. Bug fix. area:core @termuijs/core type:testing +10 pts. Tests. and removed type:bug +10 pts. Bug fix. labels Jun 30, 2026
@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

In Renderer._flush(), _lastStyleFingerprint is moved from before the row loop to inside it, so the style suppression state resets at each row boundary during differential rendering. A new test verifies that ANSI reset sequences are emitted at row transitions when adjacent rows share the same style fingerprint.

Diff Renderer Row Boundary Fix

Layer / File(s) Summary
Reset style fingerprint per row + test
packages/core/src/terminal/Renderer.ts, packages/core/src/terminal/Renderer.test.ts
_lastStyleFingerprint = null is moved inside the for loop in _flush(). A new test renders two rows with matching styles and asserts \x1b[0m appears at least twice in the output.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Possibly related PRs

  • Karanjot786/TermUI#709: Modifies Renderer._flush() diff-rendering behavior around per-row style fingerprints, overlapping directly with this fix.
  • Karanjot786/TermUI#954: Changes _lastStyleFingerprint reset and cell-level style fingerprinting in the same renderer logic as this PR.

Suggested labels

type:bug, level:beginner

Suggested reviewers

  • Karanjot786

Poem

🐇 A fingerprint clung past its row,
Painting wrong colors in the glow.
One null per line, placed just right,
Resets the style, restores the light—
Each row now fresh, a clean new show! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The core description is clear, but the template-required Which package(s) and Type of Change sections are missing. Add the missing template sections, especially Which package(s), Type of Change, and any checklist/notes items you completed.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is specific, concise, and accurately describes the row-boundary style fingerprint fix.
Linked Issues check ✅ Passed The code and test directly address #1910 by resetting the fingerprint per row and verifying reset sequences across rows.
Out of Scope Changes check ✅ Passed The change stays focused on the renderer fix and its regression test, with no unrelated code paths added.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the type:bug +10 pts. Bug fix. label Jun 30, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/core/src/terminal/Renderer.test.ts`:
- Around line 251-263: The regression test in Renderer.test.ts is too weak
because the current reset count can still pass when row 1 does not re-apply
styles after its moveTo call. Tighten the assertion around Renderer.renderNow()
by checking for a reset/style sequence immediately after the row-1 cursor
movement, or by requiring the extra reset that only appears when row 1 correctly
reapplies formatting, so the test fails against the buggy Renderer._flush()
behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c60b33f3-9495-4d02-8de0-4ccdda46cc81

📥 Commits

Reviewing files that changed from the base of the PR and between 85e8d36 and 198115e.

📒 Files selected for processing (2)
  • packages/core/src/terminal/Renderer.test.ts
  • packages/core/src/terminal/Renderer.ts

Comment thread packages/core/src/terminal/Renderer.test.ts
@ionfwsrijan

Copy link
Copy Markdown
Contributor Author

@Karanjot786 Please review this

@Karanjot786 Karanjot786 added gssoc:approved Approved PR. Earns +50 base points. quality:clean x 1.2 multiplier. Clean implementation. level:advanced +55 pts. Complex task. labels Jul 3, 2026
@Karanjot786 Karanjot786 merged commit e74c2bb into Karanjot786:main Jul 3, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:core @termuijs/core gssoc:approved Approved PR. Earns +50 base points. level:advanced +55 pts. Complex task. quality:clean x 1.2 multiplier. Clean implementation. type:bug +10 pts. Bug fix. type:testing +10 pts. Tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug] Style fingerprint persists across row boundaries causing visual corruption on multi-row diff renders

2 participants